fix: Update Search Index after Tag update#530
fix: Update Search Index after Tag update#530jesperhodge wants to merge 9 commits intoopenedx:mainfrom
Conversation
|
Thanks for the pull request, @jesperhodge! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
Technical FindingsHere are some helpful insights from our slack conversation as to how this can be implemented. I'm just going to post the conversation here. "What repo does meilisearch originate from? |
Reset the requirements to their state before I ran make upgrade, thus fixing version upgrade problems; and then run make compile-requirements to install openedx-events without upgrading other packages
|
Note to reviewers (e.g. @bradenmacdonald ): I'll implement this as an async celery task now and add a few small performance improvements. |
ormsbee
left a comment
There was a problem hiding this comment.
A couple of minor requests. Also, please bump the version for the repo and rebase + squash your commits. Thank you!
|
|
||
| @shared_task | ||
| def emit_content_object_associations_changed_for_tag_task(tag_id: int) -> int: | ||
| """Emit content association changed events for all objects linked to the given tag id.""" |
There was a problem hiding this comment.
Please use the docstring to expand a little on what's going on here, and how it fits into the bigger picture. It might not be intuitive to folks that we're doing this for the benefit of search indexing that is happening at a higher level in openedx-platform.
| return | ||
|
|
||
| transaction.on_commit( | ||
| lambda: emit_content_object_associations_changed_for_tag_task.delay(tag_id) |
There was a problem hiding this comment.
This code looks correct, but as a general practice, please use partials instead lambdas for this kind of thing: https://adamj.eu/tech/2022/08/22/use-partial-with-djangos-transaction-on-commit/

Description
This implements a fix for openedx/modular-learning#258 .
The issue was that after you rename a tag on a taxonomy from the taxonomy frontend in Course Authoring, the Meilisearch search index for associated tagged object does not get updated. That results in some outdated data being shown in the libraries -> tags search filter.
Use of AI
I used AI to implement some of the code, especially the introduction of the Celery task. I used it to help with tests as well, but carefully reviewed everything, and followed a red-green TDD process with the tests to ensure correctness.
I only use AI for support in targeted spots rather than have an agent just write code for me.
I have tested manually as well.
Testing Instructions